Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate unique names for monitors #666

Merged
merged 14 commits into from
Jan 6, 2025
Merged

fix: validate unique names for monitors #666

merged 14 commits into from
Jan 6, 2025

Conversation

andygodish
Copy link
Contributor

Description

Added a new piece to the pepr operator's validator function. The new functionality loops through the Package CR's monitor array, adding each description field (or a dummy string in the event of no provided description) to a set, failing given a duplicate.

The sanitizeResourceName() function was used in the validator since its also used downstream to create the ServiceMonitor resource name based on the string from the monitor.description field.

...

Related Issue

Fixes #656

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@andygodish andygodish requested a review from a team as a code owner August 13, 2024 16:51
Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't had a chance to fully review the code yet but to fix the one CI failure you'll want to rename your PR title to follow a conventional commit syntax, something like fix: validate unique names for monitors

@andygodish andygodish changed the title Monitor validate fix: validate unique names for monitors Aug 21, 2024
@andygodish andygodish marked this pull request as draft August 26, 2024 15:21
@mjnagel mjnagel marked this pull request as ready for review January 6, 2025 18:15
Copy link
Contributor

@UnicornChance UnicornChance left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mjnagel mjnagel enabled auto-merge (squash) January 6, 2025 22:43
@mjnagel mjnagel merged commit 80e28c1 into main Jan 6, 2025
15 checks passed
@mjnagel mjnagel deleted the monitor-validate branch January 6, 2025 23:15
mjnagel pushed a commit that referenced this pull request Jan 15, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.34.0](v0.33.1...v0.34.0)
(2025-01-15)


### Features

* add additional outputs to `debug-output` action
([#1073](#1073))
([29f12b4](29f12b4))
* istio native sidecars
([#1032](#1032))
([e07c6dc](e07c6dc))


### Bug Fixes

* add missing resource type `package` to `kubectl describe` failed…
([#1182](#1182))
([4236b3a](4236b3a))
* attempt fix token permissions
([#1155](#1155))
([5a46e41](5a46e41))
* remove unnecessary docker command in dev docs task
([#1180](#1180))
([9906a09](9906a09))
* validate unique names for monitors
([#666](#666))
([80e28c1](80e28c1))


### Miscellaneous

* add base url field for sso clients
([#1177](#1177))
([39bef00](39bef00))
* add dev task for docs site
([#1173](#1173))
([b0c4bc0](b0c4bc0))
* **deps:** bump cross-spawn from 7.0.3 to 7.0.6
([#1157](#1157))
([11ddada](11ddada))
* **deps:** update grafana to v1.29.0
([#1167](#1167))
([3b31358](3b31358))
* **deps:** update istio to v1.24.2
([#1135](#1135))
([0f9552a](0f9552a))
* **deps:** update keycloak to v26.0.8
([#1171](#1171))
([1346f7b](1346f7b))
* **deps:** update loki memcached to v1.6.34
([#1148](#1148))
([8bbf6b3](8bbf6b3))
* **deps:** update pepr to v0.42.3
([#1158](#1158))
([55e8a4e](55e8a4e))
* **deps:** update pepr to v15.3.0
([#1151](#1151))
([153b7e1](153b7e1))
* **deps:** update prometheus-stack
([#1137](#1137))
([8dc0781](8dc0781))
* **deps:** update prometheus-stack
([#1169](#1169))
([71cab01](71cab01))
* **deps:** update prometheus-stack to v67.9.0
([#1161](#1161))
([067df1b](067df1b))
* **deps:** update prometheus-stack to v68.1.0
([#1176](#1176))
([7088e78](7088e78))
* **deps:** update support-deps
([#1147](#1147))
([cf1a60b](cf1a60b))
* **deps:** update support-deps
([#1160](#1160))
([6c55f6b](6c55f6b))
* **deps:** update vector
([#1165](#1165))
([abb9584](abb9584))
* **deps:** update velero
([#1150](#1150))
([29ee12b](29ee12b))
* docs update issue template
([#1163](#1163))
([21486f9](21486f9))
* **docs:** add doc on non-http ingress
([#1166](#1166))
([0783525](0783525))
* **docs:** change .md link format to adhere to checker
([#1181](#1181))
([125a03b](125a03b))
* **docs:** update Flavor Specific Development Notes
([#1153](#1153))
([bba5a71](bba5a71))


### Documentation

* add note on minimum k3d version, update cli version reference
([#1174](#1174))
([c4dda4e](c4dda4e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package CR fails to properly create servicemonitor resources when looping through monitor array
3 participants